-
Notifications
You must be signed in to change notification settings - Fork 30k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
http2: expand list of known headers #15434
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but it would be helpful (albeit tedious) if the commit message could list the RFCs/Specs that each of these new items belongs to.
No problem, will definitely get it updated in the next day or two. I had the same thought earlier but figured I would at least start the review process for it. |
What about |
Add access-control-*, dnt, forwarded, trailer, tk, upgrade-insecure-requests, warning, x-content-type-options and x-frame-options to known list of headers for HTTP2. Expand tests to account for these headers. Fixes: nodejs#15337 Refs: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers Refs: https://www.w3.org/TR/cors/#syntax Refs: https://www.w3.org/2011/tracking-protection/drafts/tracking-dnt.html#dnt-header-field Refs: https://tools.ietf.org/html/rfc7239#section-4 Refs: https://tools.ietf.org/html/rfc7230#section-4.4 Refs: https://www.w3.org/2011/tracking-protection/drafts/tracking-dnt.html#response-header-field Refs: https://www.w3.org/TR/upgrade-insecure-requests/#preference Refs: https://tools.ietf.org/html/rfc7234#section-5.5 Refs: https://fetch.spec.whatwg.org/#x-content-type-options-header Refs: https://tools.ietf.org/html/rfc7034
ca9b35f
to
88a5c14
Compare
Updated to include Trailer & also added Refs for all the different specs to the commit message. This should be ready for a CI run. |
Failures don't seem related as far as I can tell. I've seen the linter fail a few times recently even though it doesn't seem to indicate any errors being present? |
Add access-control-*, dnt, forwarded, trailer, tk, upgrade-insecure-requests, warning, x-content-type-options and x-frame-options to known list of headers for HTTP2. Expand tests to account for these headers. Fixes: #15337 Refs: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers Refs: https://www.w3.org/TR/cors/#syntax Refs: https://www.w3.org/2011/tracking-protection/drafts/tracking-dnt.html#dnt-header-field Refs: https://tools.ietf.org/html/rfc7239#section-4 Refs: https://tools.ietf.org/html/rfc7230#section-4.4 Refs: https://www.w3.org/2011/tracking-protection/drafts/tracking-dnt.html#response-header-field Refs: https://www.w3.org/TR/upgrade-insecure-requests/#preference Refs: https://tools.ietf.org/html/rfc7234#section-5.5 Refs: https://fetch.spec.whatwg.org/#x-content-type-options-header Refs: https://tools.ietf.org/html/rfc7034 PR-URL: #15434 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Landed in 2f5bef4 |
Add access-control-*, dnt, forwarded, trailer, tk, upgrade-insecure-requests, warning, x-content-type-options and x-frame-options to known list of headers for HTTP2. Expand tests to account for these headers. Fixes: #15337 Refs: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers Refs: https://www.w3.org/TR/cors/#syntax Refs: https://www.w3.org/2011/tracking-protection/drafts/tracking-dnt.html#dnt-header-field Refs: https://tools.ietf.org/html/rfc7239#section-4 Refs: https://tools.ietf.org/html/rfc7230#section-4.4 Refs: https://www.w3.org/2011/tracking-protection/drafts/tracking-dnt.html#response-header-field Refs: https://www.w3.org/TR/upgrade-insecure-requests/#preference Refs: https://tools.ietf.org/html/rfc7234#section-5.5 Refs: https://fetch.spec.whatwg.org/#x-content-type-options-header Refs: https://tools.ietf.org/html/rfc7034 PR-URL: #15434 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Add access-control-*, dnt, forwarded, trailer, tk, upgrade-insecure-requests, warning, x-content-type-options and x-frame-options to known list of headers for HTTP2. Expand tests to account for these headers. Fixes: nodejs/node#15337 Refs: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers Refs: https://www.w3.org/TR/cors/#syntax Refs: https://www.w3.org/2011/tracking-protection/drafts/tracking-dnt.html#dnt-header-field Refs: https://tools.ietf.org/html/rfc7239#section-4 Refs: https://tools.ietf.org/html/rfc7230#section-4.4 Refs: https://www.w3.org/2011/tracking-protection/drafts/tracking-dnt.html#response-header-field Refs: https://www.w3.org/TR/upgrade-insecure-requests/#preference Refs: https://tools.ietf.org/html/rfc7234#section-5.5 Refs: https://fetch.spec.whatwg.org/#x-content-type-options-header Refs: https://tools.ietf.org/html/rfc7034 PR-URL: nodejs/node#15434 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Add access-control-*, dnt, forwarded, trailer, tk, upgrade-insecure-requests, warning, x-content-type-options and x-frame-options to known list of headers for HTTP2. Expand tests to account for these headers. Fixes: nodejs/node#15337 Refs: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers Refs: https://www.w3.org/TR/cors/#syntax Refs: https://www.w3.org/2011/tracking-protection/drafts/tracking-dnt.html#dnt-header-field Refs: https://tools.ietf.org/html/rfc7239#section-4 Refs: https://tools.ietf.org/html/rfc7230#section-4.4 Refs: https://www.w3.org/2011/tracking-protection/drafts/tracking-dnt.html#response-header-field Refs: https://www.w3.org/TR/upgrade-insecure-requests/#preference Refs: https://tools.ietf.org/html/rfc7234#section-5.5 Refs: https://fetch.spec.whatwg.org/#x-content-type-options-header Refs: https://tools.ietf.org/html/rfc7034 PR-URL: nodejs/node#15434 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This PR adds access-control-*, dnt, forwarded, tk, upgrade-insecure-requests, warning, x-content-type-options and x-frame-options to known list of headers for HTTP2. Also expands tests to account for these headers. Should address #15337
All of the headers added have an official proposal/standard and are in active use. There might be more that I missed since they're kind of scattered all over and there's no one perfect list.
The most impactful thing here is the validation for new single value headers (since adding things that aren't single value headers to that list would break users' code):
forwardedUpdate: this is clearly not single value (this is just a list error here, the code doesn't make the same mistake)Let me know if any changes are needed. Thanks!
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
http2, test